Fix macOS itk wheel size explicitly removing _skbuild directory#256
Merged
thewtex merged 2 commits intoInsightSoftwareConsortium:masterfrom Jul 17, 2023
Conversation
This commit works around an issue with the "clean" command (see [1]) that was not properly considering the current MACOSX_DEPLOYMENT_TARGET env. variable. It does by explicitly deleting the scikit-build directory without relying on the "setup.py clean" command not respecting the selected deployment target. Considering that 1. the libraries installed while building the dependent itk-* wheels were accumulated in the install directory (e.g `_skbuild/macosx-X.Y-<arch>-3.9/setuptools/lib`) 2. the wheel is generated by systematically archiving the content of the install directory. The manifest files are used to select which file to "copy" into the install directory but these are not used by the function creating the wheel archive. See [2] 3. following scikit-build/scikit-build@ac9648bf4 (post 0.8.1), the platform name was properly considered on macOS *only* when building the wheel (using "setup.py build") but not when cleaning the build directory (using "setup.py clean") ... all the wheels depending on "itk-core" where unexpectedly large because there are including the cumulative set of files from all the wheels built beforehand. [1] scikit-build/scikit-build#1012 [2] https://github.com/pypa/wheel/blob/0.40.0/src/wheel/wheelfile.py#L121-L141 Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com> Co-authored-by: Matt McCormick <matt.mccormick@kitware.com>
Contributor
Author
|
@thewtex Builds using these branches have been started on both |
dzenanz
approved these changes
Jul 17, 2023
Member
|
Looks good on a glance, but preferably someone else should review too. |
Contributor
Author
|
For future reference, here is the comment from @thewtex
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit works around an issue with the "clean" command (see [1]) that was not properly considering the current MACOSX_DEPLOYMENT_TARGET env. variable.
It does by explicitly deleting the scikit-build directory without relying on the "setup.py clean" command not respecting the selected deployment target.
Considering that
the libraries installed while building the dependent itk-* wheels were accumulated in the install directory (e.g
_skbuild/macosx-X.Y-<arch>-3.9/setuptools/lib)the wheel is generated by systematically archiving the content of the install directory. The manifest files are used to select which file to "copy" into the install directory but these are not used by the function creating the wheel archive. See [2]
following scikit-build/scikit-build@ac9648bf4 (post 0.8.1), the platform name was properly considered on macOS only when building the wheel (using "setup.py build") but not when cleaning the build directory (using "setup.py clean")
... all the wheels depending on "itk-core" where unexpectedly large because there are including the cumulative set of files from all the wheels built beforehand.
[1] scikit-build/scikit-build#1012
[2] https://github.com/pypa/wheel/blob/0.40.0/src/wheel/wheelfile.py#L121-L141